feat: add sort commands #357
Conversation
Walkthrough这次更新引入了一个新的 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client
participant Server
participant SortCmd
User->>Client: 发送排序命令
Client->>Server: 转发排序命令
Server->>SortCmd: 调用 `SortCmd::DoCmd`
SortCmd->Server: 执行排序操作
Server->>Client: 返回排序结果
Client->>User: 显示排序结果
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
src/cmd_admin.h (1)
177-193: 新增SortCmd类
SortCmd继承自BaseCmd，适用于处理排序命令。DoInitial和DoCmd方法分别用于初始化和执行命令。lookupKeyByPattern方法可能用于支持模式匹配键查找。- 内部结构
RedisSortObject用于存储排序操作的中间结果。请检查类的设计是否符合项目的整体架构和代码风格。此外，确认这个类的实现是否满足了所有预期的功能需求。
考虑增加更多注释来解释
SortCmd类的工作原理及其方法的具体用途，这将有助于其他开发者更好地理解和维护代码。
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- src/base_cmd.h (1 hunks)
- src/cmd_admin.cc (2 hunks)
- src/cmd_admin.h (2 hunks)
- src/cmd_set.cc (1 hunks)
- src/cmd_table_manager.cc (1 hunks)
- src/cmd_zset.cc (1 hunks)
- tests/admin_test.go (1 hunks)
Additional comments not posted (11)
src/cmd_admin.h (1)
10-11: 新增头文件<optional>和<variant>这些头文件的引入是为了支持
SortCmd类中使用std::optional和std::variant。请确认这些类型在其他地方没有被重复引入，以避免头文件重复包含。src/cmd_table_manager.cc (1)
60-60: 在命令表中注册Sort命令确保
Sort命令的参数数量-2是正确的。这通常表示命令接受两个以上的参数，但具体数量可能会根据实际情况变化。请验证这一设置是否符合Sort命令的设计。Verification successful
在命令表中注册
Sort命令
Sort命令的参数数量设置为-2是正确的。这表示该命令接受两个或更多的参数，这与Sort命令的设计和功能相符。
src/cmd_admin.cc行 267-270:SortCmd构造函数接受name和arity参数，并将arity设置为-2。Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 验证 Sort 命令的参数数量设置是否正确 grep -rn "SortCmd" ./src/Length of output: 484
Script:
#!/bin/bash # Inspect the SortCmd constructor and usage to confirm the arity value grep -A 5 "SortCmd::SortCmd" ./src/cmd_admin.ccLength of output: 281
tests/admin_test.go (3)
163-186: 测试Sort命令的基本功能这些测试用例检查了
Sort命令对列表中的元素进行排序的能力。请确保测试覆盖了所有重要的边界条件和潜在的错误情况。同时，验证测试用的数据（例如列表中的元素）是否设置正确。
188-221: 测试Sort命令的Get和Store选项这些测试用例涉及到使用
Get和Store选项的更复杂场景。请检查这些选项的实现是否符合预期的行为，特别是在处理不存在的键或错误参数时的行为。
223-253: 测试Sort命令的Store功能这个测试用例验证了
Sort命令结合Store选项将排序结果存储到另一个键中的功能。确保测试检查了存储操作的正确性和结果的有效性。src/cmd_set.cc (1)
161-165: 优化SCard命令的结果处理这个更改优化了
SCard命令的结果处理逻辑，使其能够在找不到键时也返回结果。请确保这种处理方式符合业务逻辑和用户期望。src/base_cmd.h (1)
91-91: 新增了常量kCmdNameSort来表示 "sort" 命令这个更改是为了支持新的
sort命令，确保命令名称在整个项目中统一管理，有助于维护和后续的功能扩展。src/cmd_admin.cc (3)
267-268: 新增SortCmd类构造函数此构造函数正确地初始化了基类
BaseCmd，设置了命令名称、参数数量、命令标志和ACL类别。这是标准的初始化过程，确保了命令的基本属性被正确设置。
270-272:SortCmd::DoInitial方法的实现此方法设置了命令的关键字，这是处理排序命令的第一步。此实现符合预期的初始化行为，没有发现逻辑错误。
441-474:SortCmd::lookupKeyByPattern方法的实现此方法用于根据模式查找键值，逻辑正确处理了模式匹配和字段提取。此方法是
sort命令功能的重要组成部分，正确地实现了预期功能。src/cmd_zset.cc (1)
367-369: 优化布尔变量的声明和初始化在
ZRangebyscoreCmd::DoCmd方法中，布尔型变量left_close、right_close和with_scores被分别声明并初始化为true。这种分开声明的方式增加了代码的可读性和可维护性。
src/cmd_admin.cc
Outdated
| storage::Status s = PSTORE.GetBackend(client->GetCurrentDB())->GetStorage()->SMembers(client->Key(), &ret); | ||
| } else if (types[0] == "zset") { | ||
| std::vector<storage::ScoreMember> score_members; | ||
| storage::Status s = | ||
| PSTORE.GetBackend(client->GetCurrentDB())->GetStorage()->ZRange(client->Key(), 0, -1, &score_members); | ||
| char buf[32]; | ||
| int64_t score_len = 0; | ||
|
|
||
| for (auto& c : score_members) { | ||
| ret.emplace_back(c.member); | ||
| } | ||
| } else { | ||
| client->SetRes(CmdRes::kErrOther, "WRONGTYPE Operation against a key holding the wrong kind of value"); | ||
| return; | ||
| } | ||
|
|
||
| std::vector<RedisSortObject> sort_ret(ret.size()); | ||
| for (size_t i = 0; i < ret.size(); ++i) { | ||
| sort_ret[i].obj = ret[i]; | ||
| } | ||
|
|
||
| if (!dontsort) { | ||
| for (size_t i = 0; i < ret.size(); ++i) { | ||
| std::string byval; | ||
| if (!sortby.empty()) { | ||
| auto lookup = lookupKeyByPattern(client, sortby, ret[i]); | ||
| if (!lookup.has_value()) { | ||
| byval = ret[i]; | ||
| } else { | ||
| byval = std::move(lookup.value()); | ||
| } | ||
| } else { | ||
| byval = ret[i]; | ||
| } | ||
|
|
||
| if (alpha) { | ||
| sort_ret[i].u = byval; | ||
| } else { | ||
| // auto double_byval = pstd::String2d() | ||
| double double_byval; | ||
| if (pstd::String2d(byval, &double_byval)) { | ||
| sort_ret[i].u = double_byval; | ||
| } else { | ||
| client->SetRes(CmdRes::kErrOther, "One or more scores can't be converted into double"); | ||
| return; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| std::sort(sort_ret.begin(), sort_ret.end(), [&alpha, &desc](const RedisSortObject& a, const RedisSortObject& b) { | ||
| if (alpha) { | ||
| std::string score_a = std::get<std::string>(a.u); | ||
| std::string score_b = std::get<std::string>(b.u); | ||
| return !desc ? score_a < score_b : score_a > score_b; | ||
| } else { | ||
| double score_a = std::get<double>(a.u); | ||
| double score_b = std::get<double>(b.u); | ||
| return !desc ? score_a < score_b : score_a > score_b; | ||
| } | ||
| }); | ||
|
|
||
| size_t sort_size = sort_ret.size(); | ||
|
|
||
| count = count >= 0 ? count : sort_size; | ||
| offset = (offset >= 0 && offset < sort_size) ? offset : sort_size; | ||
| count = (offset + count < sort_size) ? count : sort_size - offset; | ||
|
|
||
| size_t m_start = offset; | ||
| size_t m_end = offset + count; | ||
|
|
||
| ret.clear(); | ||
| if (get_patterns.empty()) { | ||
| get_patterns.emplace_back("#"); | ||
| } | ||
|
|
||
| for (; m_start < m_end; m_start++) { | ||
| for (const std::string& pattern : get_patterns) { | ||
| std::optional<std::string> val = lookupKeyByPattern(client, pattern, sort_ret[m_start].obj); | ||
| if (val.has_value()) { | ||
| ret.push_back(val.value()); | ||
| } else { | ||
| ret.emplace_back(""); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (store_key.empty()) { | ||
| client->AppendStringVector(ret); | ||
| } else { | ||
| // std::vector<std::string> list_values(client->argv_.begin() + 2, client->argv_.end()); | ||
| uint64_t reply_num = 0; | ||
| storage::Status s = PSTORE.GetBackend(client->GetCurrentDB())->GetStorage()->RPush(store_key, ret, &reply_num); | ||
| if (s.ok()) { | ||
| client->AppendInteger(reply_num); | ||
| } else { | ||
| client->SetRes(CmdRes::kSyntaxErr, "rpush cmd error"); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
SortCmd::DoCmd 方法的复杂实现
这个方法实现了 sort 命令的核心功能，包括解析命令参数和执行排序操作。代码较长且包含多个分支，建议在未来的版本中考虑将其拆分成更小的函数，以提高代码的可读性和可维护性。
- void SortCmd::DoCmd(PClient* client) {
+ // 建议拆分此方法，将参数解析、排序逻辑、结果存储等分别封装到独立的私有方法中。Committable suggestion was skipped due to low confidence.
|
ci 失败，把 go test 时间改为 15 分钟 |
ci fails, change go test time to 15 minutes |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/pikiwidb.yml (2 hunks)
Files skipped from review due to trivial changes (1)
- .github/workflows/pikiwidb.yml
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/cmd_admin.cc (2 hunks)
Additional comments not posted (3)
src/cmd_admin.cc (3)
267-268: 构造函数SortCmd::SortCmd看起来不错。代码遵循了其他命令类的初始化模式。
270-273: 方法SortCmd::DoInitial看起来不错。代码遵循了其他命令类的初始化模式。
437-470: 方法SortCmd::lookupKeyByPattern看起来不错。代码结构良好，并使用了可选值来处理查找可能失败的情况。
src/cmd_admin.cc
Outdated
| if (types[0] == "list") { | ||
| storage::Status s = PSTORE.GetBackend(client->GetCurrentDB())->GetStorage()->LRange(client->Key(), 0, -1, &ret); | ||
| } else if (types[0] == "set") { | ||
| storage::Status s = PSTORE.GetBackend(client->GetCurrentDB())->GetStorage()->SMembers(client->Key(), &ret); | ||
| } else if (types[0] == "zset") { | ||
| std::vector<storage::ScoreMember> score_members; | ||
| storage::Status s = | ||
| PSTORE.GetBackend(client->GetCurrentDB())->GetStorage()->ZRange(client->Key(), 0, -1, &score_members); | ||
| char buf[32]; | ||
| int64_t score_len = 0; | ||
|
|
||
| for (auto& c : score_members) { | ||
| ret.emplace_back(c.member); | ||
| } | ||
| } else { | ||
| client->SetRes(CmdRes::kErrOther, "WRONGTYPE Operation against a key holding the wrong kind of value"); | ||
| return; | ||
| } | ||
|
|
||
| std::vector<RedisSortObject> sort_ret(ret.size()); | ||
| for (size_t i = 0; i < ret.size(); ++i) { | ||
| sort_ret[i].obj = ret[i]; | ||
| } | ||
|
|
||
| if (!dontsort) { | ||
| for (size_t i = 0; i < ret.size(); ++i) { | ||
| std::string byval; | ||
| if (!sortby.empty()) { | ||
| auto lookup = lookupKeyByPattern(client, sortby, ret[i]); | ||
| if (!lookup.has_value()) { | ||
| byval = ret[i]; | ||
| } else { | ||
| byval = std::move(lookup.value()); | ||
| } | ||
| } else { | ||
| byval = ret[i]; | ||
| } | ||
|
|
||
| if (alpha) { | ||
| sort_ret[i].u = byval; | ||
| } else { | ||
| double double_byval; | ||
| if (pstd::String2d(byval, &double_byval)) { | ||
| sort_ret[i].u = double_byval; | ||
| } else { | ||
| client->SetRes(CmdRes::kErrOther, "One or more scores can't be converted into double"); | ||
| return; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| std::sort(sort_ret.begin(), sort_ret.end(), [&alpha, &desc](const RedisSortObject& a, const RedisSortObject& b) { | ||
| if (alpha) { | ||
| std::string score_a = std::get<std::string>(a.u); | ||
| std::string score_b = std::get<std::string>(b.u); | ||
| return !desc ? score_a < score_b : score_a > score_b; | ||
| } else { | ||
| double score_a = std::get<double>(a.u); | ||
| double score_b = std::get<double>(b.u); | ||
| return !desc ? score_a < score_b : score_a > score_b; | ||
| } | ||
| }); | ||
|
|
||
| size_t sort_size = sort_ret.size(); | ||
|
|
||
| count = count >= 0 ? count : sort_size; | ||
| offset = (offset >= 0 && offset < sort_size) ? offset : sort_size; | ||
| count = (offset + count < sort_size) ? count : sort_size - offset; | ||
|
|
||
| size_t m_start = offset; | ||
| size_t m_end = offset + count; | ||
|
|
||
| ret.clear(); | ||
| if (get_patterns.empty()) { | ||
| get_patterns.emplace_back("#"); | ||
| } | ||
|
|
||
| for (; m_start < m_end; m_start++) { | ||
| for (const std::string& pattern : get_patterns) { | ||
| std::optional<std::string> val = lookupKeyByPattern(client, pattern, sort_ret[m_start].obj); | ||
| if (val.has_value()) { | ||
| ret.push_back(val.value()); | ||
| } else { | ||
| ret.emplace_back(""); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (store_key.empty()) { | ||
| client->AppendStringVector(ret); | ||
| } else { | ||
| uint64_t reply_num = 0; | ||
| storage::Status s = PSTORE.GetBackend(client->GetCurrentDB())->GetStorage()->RPush(store_key, ret, &reply_num); | ||
| if (s.ok()) { | ||
| client->AppendInteger(reply_num); | ||
| } else { | ||
| client->SetRes(CmdRes::kSyntaxErr, "rpush cmd error"); | ||
| } | ||
| } |
There was a problem hiding this comment.
方法 SortCmd::DoCmd 实现复杂，建议重构。
该方法实现了 sort 命令的核心功能，包括解析命令参数和执行排序操作。代码较长且包含多个分支，建议将其拆分成更小的函数，以提高代码的可读性和可维护性。
- void SortCmd::DoCmd(PClient* client) {
+ // 建议拆分此方法，将参数解析、排序逻辑、结果存储等分别封装到独立的私有方法中。Committable suggestion was skipped due to low confidence.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/pikiwidb.yml (2 hunks)
Additional comments not posted (2)
.github/workflows/pikiwidb.yml (2)
46-47: 更改已批准增加
ulimit -n 4096和go test -timeout 15m是为了确保在 macOS 上进行测试时有足够的文件描述符限制和测试时间。
71-71: 更改已批准将
go test的超时时间增加到 15 分钟，以确保测试有足够的时间完成。
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/pikiwidb.yml (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- .github/workflows/pikiwidb.yml
src/cmd_admin.cc
Outdated
| storage::Status s = PSTORE.GetBackend(client->GetCurrentDB())->GetStorage()->LRange(client->Key(), 0, -1, &ret); | ||
| } else if (types[0] == "set") { | ||
| storage::Status s = PSTORE.GetBackend(client->GetCurrentDB())->GetStorage()->SMembers(client->Key(), &ret); | ||
| } else if (types[0] == "zset") { |
There was a problem hiding this comment.
zset本身具有有序性，再进行排序会不会不合适
我看sort 命令支持zset数据类型 应该是想通过关联其他数据进行排序，就是by后面的pattern。例如：sort myzset by user*
| client->SetRes(CmdRes::kErrOther, "WRONGTYPE Operation against a key holding the wrong kind of value"); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
要是获取数据失败了怎么办，这里可以根据status加一个错误处理
嗯嗯，感谢，我再修改一下
|
ref #30 |
ref #30 |
src/cmd_admin.cc
Outdated
| storage::Status s = PSTORE.GetBackend(client->GetCurrentDB())->GetStorage()->LRange(client->Key(), 0, -1, &ret); | ||
| } else if (types[0] == "set") { | ||
| storage::Status s = PSTORE.GetBackend(client->GetCurrentDB())->GetStorage()->SMembers(client->Key(), &ret); | ||
| } else if (types[0] == "zset") { |
| return; | ||
| } | ||
| } | ||
|
|
|
不仅加了新的feat，还附上了新的测试，很不错的pr |
Not only new features are added, but also new tests are attached. It’s a very good PR. |
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (2)
src/cmd_admin.h (2)
Line range hint
181-336:
方法SortCmd::DoInitial实现复杂，建议重构。该方法实现了
sort命令的参数解析。代码较长且包含多个分支，建议将其拆分成更小的函数，以提高代码的可读性和可维护性。- void SortCmd::DoInitial(PClient* client) { + // 建议拆分此方法，将参数解析逻辑分别封装到独立的私有方法中。
Line range hint
338-420:
方法SortCmd::DoCmd实现复杂，建议重构。该方法实现了
sort命令的核心功能，包括排序和结果存储。代码较长且包含多个分支，建议将其拆分成更小的函数，以提高代码的可读性和可维护性。- void SortCmd::DoCmd(PClient* client) { + // 建议拆分此方法，将排序逻辑和结果存储分别封装到独立的私有方法中。
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/cmd_admin.cc (2 hunks)
- src/cmd_admin.h (2 hunks)
Additional comments not posted (6)
src/cmd_admin.h (3)
177-180: LGTM!构造函数初始化
SortCmd类，无问题。
Line range hint
422-455:
LGTM!方法
lookupKeyByPattern正确处理了按模式查找键的逻辑，无问题。
Line range hint
457-467:
LGTM!方法
InitialArgument初始化了排序命令的参数，无问题。src/cmd_admin.cc (3)
267-269: LGTM!构造函数初始化
SortCmd类，无问题。
422-455: LGTM!方法
lookupKeyByPattern正确处理了按模式查找键的逻辑，无问题。
457-467: LGTM!方法
InitialArgument初始化了排序命令的参数，无问题。
src/cmd_admin.cc
Outdated
| void SortCmd::DoCmd(PClient* client) { | ||
| std::vector<RedisSortObject> sort_ret(ret.size()); | ||
| for (size_t i = 0; i < ret.size(); ++i) { | ||
| sort_ret[i].obj = ret[i]; | ||
| } | ||
|
|
||
| if (!dontsort) { | ||
| for (size_t i = 0; i < ret.size(); ++i) { | ||
| std::string byval; | ||
| if (!sortby.empty()) { | ||
| auto lookup = lookupKeyByPattern(client, sortby, ret[i]); | ||
| if (!lookup.has_value()) { | ||
| byval = ret[i]; | ||
| } else { | ||
| byval = std::move(lookup.value()); | ||
| } | ||
| } else { | ||
| byval = ret[i]; | ||
| } | ||
|
|
||
| if (alpha) { | ||
| sort_ret[i].u = byval; | ||
| } else { | ||
| double double_byval; | ||
| if (pstd::String2d(byval, &double_byval)) { | ||
| sort_ret[i].u = double_byval; | ||
| } else { | ||
| client->SetRes(CmdRes::kErrOther, "One or more scores can't be converted into double"); | ||
| return; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| std::sort(sort_ret.begin(), sort_ret.end(), [this](const RedisSortObject& a, const RedisSortObject& b) { | ||
| if (this->alpha) { | ||
| std::string score_a = std::get<std::string>(a.u); | ||
| std::string score_b = std::get<std::string>(b.u); | ||
| return !this->desc ? score_a < score_b : score_a > score_b; | ||
| } else { | ||
| double score_a = std::get<double>(a.u); | ||
| double score_b = std::get<double>(b.u); | ||
| return !this->desc ? score_a < score_b : score_a > score_b; | ||
| } | ||
| }); | ||
|
|
||
| size_t sort_size = sort_ret.size(); | ||
|
|
||
| count = count >= 0 ? count : sort_size; | ||
| offset = (offset >= 0 && offset < sort_size) ? offset : sort_size; | ||
| count = (offset + count < sort_size) ? count : sort_size - offset; | ||
|
|
||
| size_t m_start = offset; | ||
| size_t m_end = offset + count; | ||
|
|
||
| ret.clear(); | ||
| if (get_patterns.empty()) { | ||
| get_patterns.emplace_back("#"); | ||
| } | ||
|
|
||
| for (; m_start < m_end; m_start++) { | ||
| for (const std::string& pattern : get_patterns) { | ||
| std::optional<std::string> val = lookupKeyByPattern(client, pattern, sort_ret[m_start].obj); | ||
| if (val.has_value()) { | ||
| ret.push_back(val.value()); | ||
| } else { | ||
| ret.emplace_back(""); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (store_key.empty()) { | ||
| client->AppendStringVector(ret); | ||
| } else { | ||
| uint64_t reply_num = 0; | ||
| storage::Status s = PSTORE.GetBackend(client->GetCurrentDB())->GetStorage()->RPush(store_key, ret, &reply_num); | ||
| if (s.ok()) { | ||
| client->AppendInteger(reply_num); | ||
| } else { | ||
| client->SetRes(CmdRes::kErrOther, s.ToString()); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
方法 SortCmd::DoCmd 实现复杂，建议重构。
该方法实现了 sort 命令的核心功能，包括排序和结果存储。代码较长且包含多个分支，建议将其拆分成更小的函数，以提高代码的可读性和可维护性。
- void SortCmd::DoCmd(PClient* client) {
+ // 建议拆分此方法，将排序逻辑和结果存储分别封装到独立的私有方法中。Committable suggestion was skipped due to low confidence.
src/cmd_admin.cc
Outdated
| bool SortCmd::DoInitial(PClient* client) { | ||
| InitialArgument(); | ||
| client->SetKey(client->argv_[1]); | ||
| size_t argc = client->argv_.size(); | ||
| for (int i = 2; i < argc; ++i) { | ||
| int leftargs = argc - i - 1; | ||
| if (strcasecmp(client->argv_[i].data(), "asc") == 0) { | ||
| desc = 0; | ||
| } else if (strcasecmp(client->argv_[i].data(), "desc") == 0) { | ||
| desc = 1; | ||
| } else if (strcasecmp(client->argv_[i].data(), "alpha") == 0) { | ||
| alpha = 1; | ||
| } else if (strcasecmp(client->argv_[i].data(), "limit") == 0 && leftargs >= 2) { | ||
| if (pstd::String2int(client->argv_[i + 1], &offset) == 0 || pstd::String2int(client->argv_[i + 2], &count) == 0) { | ||
| client->SetRes(CmdRes::kSyntaxErr); | ||
| return false; | ||
| } | ||
| i += 2; | ||
| } else if (strcasecmp(client->argv_[i].data(), "store") == 0 && leftargs >= 1) { | ||
| store_key = client->argv_[i + 1]; | ||
| i++; | ||
| } else if (strcasecmp(client->argv_[i].data(), "by") == 0 && leftargs >= 1) { | ||
| sortby = client->argv_[i + 1]; | ||
| if (sortby.find('*') == std::string::npos) { | ||
| dontsort = 1; | ||
| } | ||
| i++; | ||
| } else if (strcasecmp(client->argv_[i].data(), "get") == 0 && leftargs >= 1) { | ||
| get_patterns.push_back(client->argv_[i + 1]); | ||
| i++; | ||
| } else { | ||
| client->SetRes(CmdRes::kSyntaxErr); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| Status s; | ||
| s = PSTORE.GetBackend(client->GetCurrentDB())->GetStorage()->LRange(client->Key(), 0, -1, &ret); | ||
| if (s.ok()) { | ||
| return true; | ||
| } else if (!s.IsNotFound()) { | ||
| client->SetRes(CmdRes::kErrOther, s.ToString()); | ||
| return false; | ||
| } | ||
|
|
||
| s = PSTORE.GetBackend(client->GetCurrentDB())->GetStorage()->SMembers(client->Key(), &ret); | ||
| if (s.ok()) { | ||
| return true; | ||
| } else if (!s.IsNotFound()) { | ||
| client->SetRes(CmdRes::kErrOther, s.ToString()); | ||
| return false; | ||
| } | ||
|
|
||
| std::vector<storage::ScoreMember> score_members; | ||
| s = PSTORE.GetBackend(client->GetCurrentDB())->GetStorage()->ZRange(client->Key(), 0, -1, &score_members); | ||
| if (s.ok()) { | ||
| for (auto& c : score_members) { | ||
| ret.emplace_back(c.member); | ||
| } | ||
| return true; | ||
| } else if (!s.IsNotFound()) { | ||
| client->SetRes(CmdRes::kErrOther, s.ToString()); | ||
| return false; | ||
| } | ||
| client->SetRes(CmdRes::kErrOther, "Unknown Type"); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
方法 SortCmd::DoInitial 实现复杂，建议重构。
该方法实现了 sort 命令的参数解析。代码较长且包含多个分支，建议将其拆分成更小的函数，以提高代码的可读性和可维护性。
- void SortCmd::DoInitial(PClient* client) {
+ // 建议拆分此方法，将参数解析逻辑分别封装到独立的私有方法中。Committable suggestion was skipped due to low confidence.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/cmd_admin.cc (2 hunks)
- src/cmd_admin.h (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/cmd_admin.cc
Additional comments not posted (7)
src/cmd_admin.h (7)
10-11: 引入新的头文件是正确的新引入的
<optional>和<variant>是SortCmd类所需的，没有问题。
177-204:SortCmd类声明正确
SortCmd类的声明符合标准实践，使用std::optional和std::variant也很合适。
179-179: 构造函数定义正确构造函数
SortCmd(const std::string& name, int16_t arity)定义简洁明了，符合预期的命令类模式。
182-182:DoInitial方法定义正确
DoInitial方法重写了基类方法，方法签名正确，符合基类接口。
185-185:DoCmd方法定义正确
DoCmd方法重写了基类方法，方法签名正确，符合基类接口。
188-188:lookupKeyByPattern方法定义正确
lookupKeyByPattern方法返回std::optional<std::string>，使用std::optional很适合用于可能找到也可能找不到匹配键的方法。
187-187:InitialArgument方法定义正确
InitialArgument方法没有参数，可能用于初始化命令参数。没有发现问题。
添加了 sort 命令 和 一些测试案例
SORT key [BY pattern] [LIMIT offset count] [GET pattern [GET pattern]] [ASC | DESC] [ALPHA] [STORE destination]
Summary by CodeRabbit
新功能
SortCmd类，提供多种排序选项，包括升序、降序、字母排序、限制等。实现对列表、集合和有序集合的数据排序功能。Bug 修复
CmdDebugSegfault::DoCmd方法，添加空指针赋值检查。测试
Chores
go test命令添加15分钟的超时设置。test_on_ubuntu任务的run步骤之前添加ulimit -n 4096。